Skip to content

Fix yarn berry project gates drifting between modes (#628, #629) - #657

Open
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
agent/fix-yarn-berry-shared-gates
Open

Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
agent/fix-yarn-berry-shared-gates

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #628
Fixes #629

Summary

Hosted mode now refuses a Yarn Berry project whose root package.json mixes CRLF and LF line endings, using redirect_yarn_berry_mixed_line_endings and writing nothing. This is the same decision vendored mode already took with vendor_yarn_berry_mixed_line_endings. Before this change, hosted mode re-rendered the manifest in its majority ending, which silently rewrote lines the user never touched and left rollback with no original bytes to restore. Both modes now run one shared set of berry project gates, so they can't drift apart again.

Root cause

Each mode implemented the berry project-level gates itself (mixed line endings, cacheKey, .yarnrc.yml compressionLevel):

  • vendored: SUPPORTED_CACHE_KEY, refuse_*, and yarn_berry_vendor_preflight
  • hosted: YARN_BERRY_SUPPORTED_CACHE_KEY, berry_cache_key, and preflight_yarn_berry_hosted

Hosted mode also imported yarnrc_compression_level from vendor. The copies had drifted. The hosted gate looked only at yarn.lock, although the hosted rewriter also edits package.json (to add resolutions).

Change

  • New formats/yarn/berry_gates.rs, a pure module with no I/O:
    • SUPPORTED_CACHE_KEY
    • one cache_key extractor
    • yarnrc_compression_level, moved here together with its tests
    • check(lock, manifest, Yarnrc) and the per-gate check_* functions, which return a BerryGate (MixedLineEndings { file }, NoMetadata, CacheKey { found }, Compression { level }, YarnrcUnreadable { error }). BerryGate owns the detail text and the code suffix.
  • Vendored: refuse_* are now thin wrappers that map a BerryGate to the vendor_yarn_berry_* codes, and NoMetadata to vendor_lockfile_version_unsupported. The codes don't change.
  • Hosted: preflight_yarn_berry_hosted(lock, manifest, yarnrc) calls berry_gates::check and maps results to the redirect_yarn_berry_* codes. All three callers now pass the root package.json:
    • the rewriter
    • the vendored→hosted takeover in scan/hosted.rs, which now refuses before reverting, wet and --dry-run
    • the hosted upstream restore in upstream/npm.rs, which already re-rendered that manifest
  • lock_inventory::yarn reads cacheKey through the same extractor. YARN_BERRY_SUPPORTED_CACHE_KEY, berry_cache_key and berry_metadata are deleted, and patch/redirect no longer imports gate code from vendor::yarn_berry_lock.
  • Docs: CLI_CONTRACT.md (the hosted berry line-endings paragraph and the takeover gates), docs/ecosystems.md, and CHANGELOG.md.

Detail text: the two modes' wording is now identical. Vendored mode's wording was kept, and it already names yarn install. A berry __metadata block with no cacheKey line now says (missing) in both modes; vendored mode used to print an empty value.

Test evidence

Issue Test Before fix After fix (759933a)
#628 rewriter patch::redirect::tests::berry_mixed_root_manifest_is_refused_untouched FAILED on main + test (nothing written: ["package.json", "yarn.lock"]) ok
#628 fresh hosted scan in_process_redirect::scan_redirect_refuses_a_mixed_line_ending_yarn_berry_manifest FAILED at 2409f1a (no redirect_yarn_berry_mixed_line_endings warning) ok
#628 vendored→hosted takeover in_process_vendor::berry_takeovers_refuse_before_reverting_the_old_mode, new "mixed package.json" leg (wet and dry) FAILED at 2409f1a (vendored→hosted mixed package.json dry=true: refused with redirect_yarn_berry_mixed_line_endings) ok
#629 one decision for both modes vendor::yarn_berry_lock::tests::both_modes_take_the_same_project_gate_decision (table: supported, BOM+CRLF, cacheKey 10, no cacheKey, compressionLevel: mixed, mixed lock, mixed manifest; asserts the same suffix and identical detail) n/a (new API) ok
#629 shared module formats::yarn::berry_gates::tests::* (4 tests plus the 3 moved yarnrc_compression_level tests) n/a ok

Local runs:

  • cargo clippy --workspace --all-features -- -D warnings: ok
  • cargo fmt: the new module is rustfmt-clean. Every changed hunk is formatted. main itself isn't cargo fmt --check clean, so I did not reformat files outside this change.
  • cargo test --workspace --all-features --no-fail-fast: 9731 passed, 12 failed. All 12 failures are permission-denial tests: *_state_write_failure_*, *_unremovable*, wire_*failure*, relax_loop_must_not_traverse_symlinked_root, redirect_json_mode_write_failures_*, partial_lockfile_write_failure_*, vlt_heal_invalidation_failure. They depend on chmod taking effect, and the sandbox runs as root, which ignores it. None of them touch yarn code. After the history cleanup, in_process_vendor (104/104), the core berry|yarn unit tests (233/233) and the in_process_redirect yarn_berry tests (5/5) were re-run.
  • scripts/yarn-berry-vex-matrix.sh 4.18.0 (with COREPACK_NPM_REGISTRY set): e2e_redirect_yarn_berry_build, e2e_vendor_yarn_berry_build, e2e_yarn4_pnpm_linker_build and e2e_yarn4_workspaces_build all pass (56 tests) on real yarn 4.18.0. In e2e_yarn_legacy_cachekey_refusal_build the yarn 3 cells pass. Its two yarn 2.4.3 cells couldn't run locally: the sandbox proxy blocks repo.yarnpkg.com, and yarn 2 isn't on the npm registry. CI covers them.

CI on 759933a: green. 485 of 491 check runs pass and 6 are skipped. Bugbot found no issues, and there are no review threads. The Poetry backtest native (ubuntu-latest, 2.0.1) failed rescanIdempotent once; this PR touches no Poetry code, and its single re-run passed.

No wrapper changes are needed: npm/, pypi/ and gem/ only dispatch to the binary.

Follow-up, not changed here: in hosted mode, an unreadable .yarnrc.yml is still treated as absent. The rewriter receives files that were already read, so it can't tell "unreadable" apart from "missing". Vendored mode refuses it with YarnrcUnreadable. The gate now supports Yarnrc::Unreadable, so the hosted takeover could adopt it in a later change.

🤖 Generated with Claude Code


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A berry project whose root package.json mixes CRLF and LF is refused
by vendored mode, but hosted mode rewrites it in the majority ending.
These tests cover a fresh hosted scan and the vendored-to-hosted
takeover (#628). They fail until the gate is shared.

Assisted-by: Claude Code:claude-opus-5-5
Hosted and vendored modes each carried their own copy of the yarn
berry project refusals (mixed line endings, cacheKey, .yarnrc.yml
compressionLevel), and the copies drifted: hosted mode never checked
the root package.json, so it silently rewrote a mixed-line-ending
manifest that vendored mode refuses (#628).

The gates now live once in formats/yarn/berry_gates.rs. The vendored
backend and its takeover preflight, the hosted rewriter, the
vendored-to-hosted takeover and the hosted restore all call it and
keep their existing codes. Hosted mode now refuses a mixed
package.json with redirect_yarn_berry_mixed_line_endings before
writing or reverting anything (#629).

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) force-pushed the agent/fix-yarn-berry-shared-gates branch from 853f810 to 759933a Compare October 3, 2026 06:08
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] native (ubuntu-latest, 2.0.1) failed on 759933a. This is the Poetry 2.0.1 backtest, and the only failing check is rescanIdempotent in the direct hosted cell. The other 4 cells pass.

I don't think this failure is caused by this PR:

  • This PR only touches the yarn berry gates, and no Poetry or PyPI code path calls them.
  • The same code (853f810, which differs only in formatting of unrelated files) passed every check suite, including this Poetry job.

PR #596 (open) targets Poetry matrix flakes caused by PyPI and patch-API transport blips, which could explain a failed hosted re-scan. I haven't confirmed that this is the same failure, so I'm not porting #596's change here; the re-run will show whether it reproduces.

A re-run of the failed job is refused with 403 while the workflow is still running. I'll re-run it once when the run completes. If it fails again, I'll treat it as real and root-cause it.


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 759933a. Configure here.

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 3, 2026
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 3, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Ready for review at head 759933a.

  • CI: green. 485 of 491 check runs pass and 6 are skipped. One Poetry 2.0.1 backtest (rescanIdempotent) flaked; this PR touches no Poetry code and its single re-run passed.
  • Bugbot: reviewed 759933a and found no issues. There are no open review threads.
  • For reviewers: the new shared formats/yarn/berry_gates.rs module, and hosted mode now refusing a mixed-line-ending root package.json (vendored→hosted takeover included) instead of re-rendering it.

The Slack announcement is pending because no Slack send tool was available in this run.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Codex review of 759933a850676e86aeb6a89742f68c926b9dcd1a: ready to merge as-is from this review. No actionable findings.

The shared Berry gates preserve the supported cache/compression checks and warning codes. Mixed root manifests are now refused before hosted rewrites, before vendored takeover reverts, and before upstream restore writes. Uniform LF/CRLF behavior and BOM preservation remain covered.

Validation:

  • 343 repository tests passed: 233 core Berry/Yarn tests, five hosted Berry CLI tests and 105 vendored CLI tests, including the wet/dry takeover regression.
  • A separate public restore test passed four scenarios: wet/dry mixed-manifest refusal with files unchanged, and wet/dry uniform CRLF/BOM success.
  • Independent native checks used Yarn 2.4.3, 3.8.7, 4.0.2 and 4.18.0. Fourteen metadata comparisons and six native line-ending/immutable controls support the implementation. Native Yarn can normalize a mixed manifest even under --immutable, confirming the reason for the explicit refusal.
  • All 12 reviewed source hashes match this commit. Independent review and the merge check against main 045d7ec7 are clear.

Fresh CI is clear: 485 successful checks, 7 skipped; 13 successful workflows and 1 skipped. Bugbot is clear on this exact commit, with no unresolved threads or outstanding actionable feedback. The documented pre-existing hosted behavior for unreadable .yarnrc.yml remains a separate follow-up.

GitHub's normal human approval requirement remains before merge.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

2 participants